URL support for parsing (issue #77) #78
Conversation
|
This would be a great help if it was integrated. Thank you! |
|
Okay noted. I will (try to make time to...) mock the active HTTP get and make what ever changes necessary to do that this weekend. Unless it's something either of you want to do? @james-elastic @Courtcircuits |
|
By the way, I can enhance this pull request. Currently, there isn't much to it. I'm considering implementing the "functional option pattern" to instantiate the HTTP client (more about this pattern : YouTube video). @arran4, I can also include a few more tests (e.g mock of HTTP connection). I recognize that the ones I added may not be entirely relevant. |
|
Thanks. The functional options pattern is fine, however just a var-args with a type swtich could do as we don't really need meta information around the http.Client. I am not sure if more tests are warranted as there isn't much logic, definitely need to test the http.Options solution. Also for people have need custom headers, ie authorization, post, etc. You might want to split it into two (or more) functions, where the http.Request is created in the Get and passed to a http.Do function, which itself is also a constructor. Please try use fmt.Errorf to wrap the errors if it adds value. |
| func TestIssue77(t *testing.T) { | ||
| url := "https://proseconsult.umontpellier.fr/jsp/custom/modules/plannings/direct_cal.jsp?data=58c99062bab31d256bee14356aca3f2423c0f022cb9660eba051b2653be722c4c7f281e4e3ad06b85d3374100ac416a4dc5c094f7d1a811b903031bde802c7f50e0bd1077f9461bed8f9a32b516a3c63525f110c026ed6da86f487dd451ca812c1c60bb40b1502b6511435cf9908feb2166c54e36382c1aa3eb0ff5cb8980cdb,1" | ||
|
|
||
| cal, err := ParseCalendarFromUrl(url) |
There was a problem hiding this comment.
This looks like it would be making a real HTTPS request in tests. It should be using a mock http client.
| Creating: | ||
| Usage, parsing from a URL : | ||
| ```golang | ||
| cal, err := ParseCalendar("an-ics-url") |
There was a problem hiding this comment.
Minor, but an example URL + a http client custom example might be nice. I've been contemplating making this more readable.
|
Resolved conflicts in #102 |
|
@Courtcircuits I created a new PR to up date this, can you please review it: #102 I will merge it (or make more changes) in the coming days if no response. |
|
Yay! |
* master: Duplication issue fixed. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # calendar_test.go
* master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # components.go
* origin/master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support
* origin/master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # calendar.go # calendar_test.go # components.go
* origin/master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # calendar.go
* Add leaks and vunerability checks * Requires secrets now * Renamed interface as it's useful own it's own. Made it public. Added a comment * fix: omit zone in "AllDay" event helpers For a date-only event (i.e., an event that lasts for the full day) the iCalendar specification indicates that the value for DTSTART / DTEND should be a DATE https://icalendar.org/iCalendar-RFC-5545/3-6-1-event-component.html > The "VEVENT" is also the calendar component used to specify an > anniversary or daily reminder within a calendar. These events have a > DATE value type for the "DTSTART" property instead of the default value > type of DATE-TIME. If such a "VEVENT" has a "DTEND" property, it MUST be > specified as a DATE value also The DATE format (https://icalendar.org/iCalendar-RFC-5545/3-3-4-date.html) should omit both time and zone/location elements and additionally notes that "The "TZID" property parameter MUST NOT be applied to DATE properties" As per the specification, this PR also adds an explicit "VALUE=DATE" parameter when the AllDay helpers were called, to indicate that the property's default value type has been overridden and the VEVENT is intended to be an all-day event https://icalendar.org/iCalendar-RFC-5545/3-2-20-value-data-types.html Finally the SetDuration call has been updated to preserve the "AllDay" characteristics if the existing start or end has been specified in DATE format, which is also a requirement of the spec. Contributes-to: arran4#55 * calendar parsing url support * usage example, README * added functionnal option pattern for url parsing * Should be able to distinguish unset from invalid time properties * Improve error for property not found Co-authored-by: Arran Ubels <arran4@gmail.com> * Remove deprecated ioutil * Move targeted Go version to 1.20 And test the target forever * Add method to remove property Fixes arran4#95 * Merged * New tool * Reintegration of arran4#67 * Resynced and moved some functions around arran4#78 * Test fixed. * Create bug.md * Create other.md * Create default.md * Rename default.md to pull_request_template.md * Minor update * refactor: remove unnecessary named return values, harmonizing code base * refactor: rename var to reflect what it is These functions take optional variadic PropertyParam arguments, in ical speak they are not properties, but parameters for a property. * refactor: use ReplaceAll * refactor: prefer switch for readability * refactor: use consistent receiver names * refactor: rename unused arg as '_' * Tests added. * Some larger upgrades. * Fix * Some multiple-ness. * Duplication issue fixed. * Merged * Merge remote-tracking branch 'origin/master' into issue97 * origin/master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around arran4#78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # calendar.go # calendar_test.go # components.go --------- Co-authored-by: Dominic Evans <dominic.evans@uk.ibm.com> Co-authored-by: tradulescu <tristan-mihai.radulescu@etu.umontpellier.fr> Co-authored-by: Bracken Dawson <abdawson@gmail.com> Co-authored-by: Daniel Lublin <daniel@lublin.se>
In response to issue #77, I have made some improvements to the codebase. Specifically, I have added a new method
ParseCalendarFromUrl(string)and a new test case that verifies the absence of errors when invoking this method.Currently, this method initializes a basic
http.Clientand forwards the response body to theParseCalendar(string)function. However, as suggested in issue #77, it may be beneficial to allow users to use a customhttp.Clientto address authentication, CORS, and similar concerns.I am currently exploring the best design approach to accommodate this feature request. Any input or suggestions on the optimal design for this enhancement would be greatly appreciated.